Optimize object forwarding for single threaded GC#1283
Optimize object forwarding for single threaded GC#1283k-sareen wants to merge 4 commits intommtk:masterfrom
Conversation
|
1.74.1 failed to compile because icu4x started to require rust 1.81. Let's fix that separately. |
wks
left a comment
wks
left a comment
There was a problem hiding this comment.
Two high-level comments:
Firstly, we can skip the BEING_FORWARDED state without affecting the callers in CopySpace and ImmixSpace. After calling attempt_to_forward, the caller always calls state_is_forwarded_or_being_forwarded to test if the return value is FORWARDING_NOT_TRIGGERED_YET or other states (BEING_FORWARDED or FORWARDED), but in fact it can never observe BEING_FORWARDED even we set the state to BEING_FORWARDED in attempt_to_forward. That's because if it is FORWARDING_NOT_TRIGGERED_YET, the (only) GC worker will immediately try to forward the object, and it will either successfully copy the object and change the state to FORWARDED, or decide not to copy the object and revert the state to FORWARDING_NOT_TRIGGERED_YET. Then a subsequent invocation of attempt_to_forward will either observe FORWARDED or FORWARDING_NOT_TRIGGERED_YET. So we can simply omit the store in attempt_to_forward.
Secondly, I think the "early exit" style is more readable than the current if-else style.
| .compare_exchange_metadata::<VM, u8>( | ||
| if old_value == FORWARDING_NOT_TRIGGERED_YET { | ||
| unsafe { | ||
| VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC.store::<VM, u8>( |
There was a problem hiding this comment.
This store can be removed. The caller of attempt_to_forward only needs the old_value. As long as it is FORWARDING_NOT_TRIGGERED_YET, the caller will immediately try to forward the object.
| // See: https://github.com/mmtk/mmtk-core/issues/579 | ||
| debug_assert!( | ||
| forwarding_bits == FORWARDING_NOT_TRIGGERED_YET, | ||
| forwarding_bits != BEING_FORWARDED, |
There was a problem hiding this comment.
We can just assert forwarding_bits == FORWARDED if we don't use BEING_FORWARDED.
| /// The successful worker will set the object forwarding bits to BEING_FORWARDED, preventing other workers from forwarding the same object. | ||
| pub fn attempt_to_forward<VM: VMBinding>(object: ObjectReference) -> u8 { | ||
| loop { | ||
| if cfg!(not(feature = "single_worker")) { |
There was a problem hiding this comment.
Having this if-else statement with not(feature = "single_worker") as the condition makes the code a bit harder to read. Since it is simpler to do it with a single worker, we can rewrite it in the early exit style. For example,
pub fn attempt_to_forward<VM: VMBinding>(object: ObjectReference) -> u8 {
if cfg!(feature = "single_worker") {
return get_forwarding_status::<VM>(object); // Assume we remove BEING_FORWARDED.
}
// old code here
}
The problem is then I think the code for the multi-threaded object forwarding will still be in the generated binary leading to code bloat. I don't know if the Rust/LLVM compiler will remove that code if the |
|
|
|
The main difference is that code under While |
3 participants
No description provided.